-
-
Notifications
You must be signed in to change notification settings - Fork 21.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Convert output of GLES2 to linear color space #51780
Conversation
cc48dcb
to
de381c6
Compare
Tested this both on desktop and on the Quest. In both it seems to do the conversions just fine. The way it looks inside of the headset is now the same between GLES2 converting sRGB -> Linear and GLES3 where we keep the results linear. It does come across slightly darker in the headset but I think this is because the OpenXR uses a slightly different color setup. It's heaps better then it looking waaaay too bright and oversaturated. |
Looks good to me from looking through the source code 👍 , I'll leave for @clayjohn to review. |
Looks great! |
I haven't tested but if there is any it should be really minor. It's just applying a gradient to the color that is output. Also keep in mind that for the XR interfaces that do display the right color, they are doing this color conversion within the lens distortion shader most likely so any loss is just moved. The "proper" way to solve this is to redo the entire 3D rendering in GLES2 in linear by converting all the color inputs. That would work great for our needs but it would make everyone elses games much slower as suddenly they need a post process to convert linear to sRGB (as we do in the GLES3 renderer). That all said, the mobile renderer in Vulkan does do this correctly. All 3D rendering happens in linear and during the tonemapping post process we optionally convert to sRGB. Here it is less of a problem because where performance is needed we can do this within a subpass. |
de381c6
to
9b9fbd1
Compare
9b9fbd1
to
73722f3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! It's a shame we have to do this, but it's one more hack we can do away with for 4.0!
Yup, and it's a defensible one that only effects VR so... |
Thanks! |
In XR the render output is sent to a compositor that will do further processing of the images and output them to an HMD, one of the adjustments made here is converting to the color space that best suits the HMD.
The problem we've run into a few times is that as a result many of the compositors expect the render images to be in linear color space.
Some like OpenVR and Oculus' VrAPI do have support for sRGB input (though it doesn't work in OpenVR when RGBA16F is used) but it looks like OpenXR only offers it through hardware sRGB buffers which don't seem to work as advertised when using GLES2. The bottom line is that OpenXR simply expects us to supply render buffers in linear color space.
For the GLES3 renderer we solved this issue a long time ago by introducing the
keep_3d_linear
switch as rendering happens in linear color space and we simply convert this to sRGB as part of our tonemapping post process. The introduced switch skips this conversion.For the GLES2 renderer however we render everything in sRGB color space and this is all we have to offer, the
keep_3d_linear
switch has no effect. As it would be way to much work to convert the GLES2 renderer to work in linear color space and it requiring a post process to then convert to sRGB this isn't really an option.This PR takes the opposite approach letting the renderer do its work in sRGB color space but converting the sRGB result to linear color space if
keep_3d_linear
is enabled.This PR also changes the arvr blit functions for creating the spectator view that if it is provided with a render buffer that has
keep_3d_linear
turned on, it converts the result to sRGB. This was a long outstanding wish I implemented while working on this anyway.I need to do some more testing but this PR is basically ready.